Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

rptest: perform full log reads with KgoVerifierSeqConsumer #11759

Merged
merged 5 commits into from
Jul 6, 2023
Merged

rptest: perform full log reads with KgoVerifierSeqConsumer #11759

merged 5 commits into from
Jul 6, 2023

Conversation

VladLazar
Copy link
Contributor

@VladLazar VladLazar commented Jun 28, 2023 •
edited
Loading

Based on #11846

Previously, whenever a KgoVerifierSequentialConsumer and a
KgoVerifierProducer ran in parallel (which a number of tests do), there
was no guarantee that the consumer would read all partitions entirely.
Whenever wait is called on the consumer, the last_pass command is
triggered without taking into account if the producer has completed.

This is problematic because it can hide stuck consumer issues. This
patch makes use of the enhanced kgo-verifier status which now contains
the high watermark for each partition. When creating a sequential
consumer, a reference to the producer can be provided. If that was the
case, when wait is called on the consumer we will wait until the
producer has finished and the high watermarks match.

Backports Required

  • none - not a bug fix
  • none - this is a backport
  • none - issue does not exist in previous branches
  • none - papercut/not impactful enough to backport
  • v23.1.x
  • v22.3.x
  • v22.2.x

Release Notes

  • none

@VladLazar VladLazar requested a review from a team as a code owner June 28, 2023 14:51
@VladLazar VladLazar requested review from ivotron and removed request for a team June 28, 2023 14:51
@VladLazar
Copy link
Contributor Author

/ci-repeat 5
release
skip-unit
dt-repeat=10
dt-log-level=trace
tests/rptest/tests/cloud_storage_timing_stress_test.py

@VladLazar VladLazar force-pushed the kgo-consumer-full-reads branch 6 times, most recently from 090b8b7 to 3d1ea57 Compare June 30, 2023 14:57
@VladLazar VladLazar requested a review from jcsp June 30, 2023 14:57
@VladLazar VladLazar force-pushed the kgo-consumer-full-reads branch 2 times, most recently from dd6cb5a to e0c6bd0 Compare July 3, 2023 09:27
@VladLazar
Copy link
Contributor Author

All failures apart from one in CloudStorageTimingStressTest were known. I've pushed an update to make the test more stable (here). Will run a ci-repeat just for it.

@VladLazar
Copy link
Contributor Author

/ci-repeat 5
release
skip-units
dt-repeat=10
tests/rptest/tests/cloud_storage_timing_stress_test.py

@VladLazar VladLazar mentioned this pull request Jul 3, 2023
Merged
7 tasks
@VladLazar
Copy link
Contributor Author

/ci-repeat 5
release
skip-units
dt-repeat=10
tests/rptest/tests/cloud_storage_timing_stress_test.py

@VladLazar
Copy link
Contributor Author

/ci-repeat 5
release
skip-units
dt-repeat=10
tests/rptest/tests/cloud_storage_timing_stress_test.py

@VladLazar
Copy link
Contributor Author

/ci-repeat 5
release
skip-units
dt-repeat=10
tests/rptest/tests/cloud_storage_timing_stress_test.py

@VladLazar
Copy link
Contributor Author

Timing tests are stable now! Once #11846, this can be rebased and merged.

@VladLazar
Copy link
Contributor Author

/ci-repeat

rptest: make kgo seq consumer read full log
a33520b 
Previously, whenever a KgoVerifierSequentialConsumer and a
KgoVerifierProducer ran in parallel (which a number of tests do), there
was no guarantee that the consumer would read all partitions entirely.
Whenever `wait` is called on the consumer, the `last_pass` command is
triggered without taking into account if the producer has completed.

This is problematic because it can hide stuck consumer issues. This
patch makes use of the enhanced kgo-verifier status which now contains
the high watermark for each partition. When creating a sequential
consumer, a reference to the producer can be provided. If that was the
case, when `wait` is called on the consumer we will wait until the
producer has finished and the high watermarks match.
Vlad Lazar added 3 commits July 4, 2023 21:26
rptest: consume full log in timing stress tests
a90ab2e
rptest: upload manifests more eagerly in timing test
2cce635 
This commit updates the SI configuration usesd for the
CloudStorageTimingStressTest(s) to upload the manifest at each
oportunity. The change should make the test more robust.
dt/docker: bump kgo-verifier version for HWMs
2893f9a
@VladLazar
Copy link
Contributor Author

/ci-repeat

2 similar comments
@abhijat
Copy link
Contributor

abhijat commented Jul 5, 2023

/ci-repeat

@abhijat
Copy link
Contributor

abhijat commented Jul 5, 2023

/ci-repeat

@abhijat
Copy link
Contributor

abhijat commented Jul 5, 2023

#11698 (https://buildkite.com/redpanda/redpanda/builds/32526#01892623-a6bf-4ba0-930a-d33a5617d84f)

#11884 (https://buildkite.com/redpanda/redpanda/builds/32526#018925de-2328-4968-949c-780edc7e43e9) - new issue but not related to these changes.

@jcsp jcsp merged commit 74d6675 into redpanda-data:dev Jul 6, 2023
This was referenced Aug 18, 2023
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jcsp jcsp jcsp approved these changes

@ivotron ivotron Awaiting requested review from ivotron ivotron is a code owner automatically assigned from redpanda-data/devprod

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@VladLazar @abhijat @jcsp